Skip to content

Review fixes: deploy/onboarding, schema & doc drift, CI hardening - #18

Merged
mousebrains merged 12 commits into
mainfrom
review-fixes
May 24, 2026
Merged

Review fixes: deploy/onboarding, schema & doc drift, CI hardening#18
mousebrains merged 12 commits into
mainfrom
review-fixes

Conversation

@mousebrains

Copy link
Copy Markdown
Owner

Remediates the findings from a deep review of the repo. Scope is the deploy-critical and drift/bloat findings; the trace-geometry work is a deliberate separate follow-up PR.

Deploy & onboarding — the #1 finding was a broken documented quick-start

  • Quick-start is now init-db --no-seed → import_metadata.py → pipeline, so a fresh DB renders real data instead of dying at orphan-check with ~300 orphan sources (README, CLAUDE.md, SETUP.md §3/§4).
  • import_metadata.py resolves its DB path from DATABASE_URL (matching levels), so it can't target a different file than init-db.
  • SETUP.md §3 creates /home/pat/DB and sets DATABASE_URL + SQLITE_PATH — fixes the "unable to open database file" a real fresh install hit (Python reads DATABASE_URL, not SQLITE_PATH).
  • install.service.sh installs all 15 timers (was missing kayak-fetch-osmb + kayak-status).
  • SETUP.md §6 deploys the live conf/sites/ + conf/snippets/ nginx split instead of the retired deploy/levels; the historical Oracle Cloud section is removed.
  • nginx-editor-env.conf $site_url is hostname-aware (post-2026-05-20 cutover).
  • Hardware specs corrected to the Hetzner CPX11 (2 vCPU / 2 GB / 40 GB) in SETUP.md and the engine.py SQLite-PRAGMA comment.

Migrations

  • Removed stray BEGIN/COMMIT from migrations 0052/0054/0055/0056 — they ran inside the migrate runner's own transaction and defeated its atomicity.

Schema & documentation drift

  • Schema docs: table count corrected to 24/25, the ghost maintainer_credential table removed (dropped back in migration 0022), gradient_profile/gradient_unreliable columns documented.
  • CLAUDE.md pipeline now lists check-reaches; systemd timer counts reconciled to 15; CHANGELOG [Unreleased] populated; CI/pre-commit PHPStan comments corrected (level 8, 79-item baseline).
  • Added docs/done/README.md index; archived the five run-once gradient scripts into docs/one-offs/ per the repo's own lifecycle rule.

CI

  • biome.json now lints all project JS/CSS via glob (closes the gap where new JS — incl. gradient-profile.js — escaped CI) and the surfaced internal-sort.js lint debt is fixed.
  • Pinned ruff in the lint job (was floating); added .github/dependabot.yml; declared numpy in [dev] so the regression test runs in any dev venv.

Removed

  • The retired deploy/levels + conf/levels.nginx monoliths (~682 drifted lines); repointed the live docs/security/ references to the conf/ split, with a note covering the dated tier-audit refs.

Validation

Exercised end-to-end on a fresh Debian 13 arm64 VM: the corrected init-db --no-seed → import_metadata → pipeline produced a populated site (orphan-check: clean, check-reaches: clean), and the PHP layer — including the gradient chart — renders. The deploy docs were followed verbatim, which is how the SETUP.md §3/§4 bug was caught and fixed.

Out of scope (separate follow-up PR)

Trace-geometry simplification (~5 m Douglas–Peucker, 11.8 → 2.3 MB), the reaches.json snapshot + import wiring, and the find_huc4reach.huc[:4] determination fix.

🤖 Generated with Claude Code

mousebrains and others added 12 commits May 24, 2026 08:30
Front-door docs had drifted from reality (flagged in the branch review):

- schema: 24 ORM tables (25 live), not 25/26; drop the maintainer_credential
  ghost (dropped in migration 0022); add reach.gradient_profile +
  gradient_unreliable to the reach table.
- pipeline: document the check-reaches step that wires in after orphan-check.
- timers: reconcile the count to 15 and add kayak-status/fetch-osmb/recap to
  the README + operations lists.
- quick-start: load the metadata snapshot so a fresh DB renders data instead
  of failing orphan-check on ~300 orphan sources.
- hardware: Hetzner CPX11 (2 vCPU/2 GB/40 GB), not the stale CPX21.
- operations: add the /_internal/status operator-dashboard row; populate the
  CHANGELOG [Unreleased] section.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CI and pre-commit comments claimed 'Level 7' grandfathering '123'
pre-existing issues. The gate is actually level 8 with a 79-error
phpstan-baseline.neon.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
These four migrations lacked @no_transaction yet carried their own
BEGIN TRANSACTION/COMMIT. migrate.py wraps each non-@no_transaction file in
one transaction and splits on ';', so the inner COMMIT ended that wrapper
early -- a failure afterward would leave data committed but the version
unstamped. Removed the explicit transaction statements (the runner owns the
transaction) and left a comment to prevent regression. UPDATEs unchanged and
idempotent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…v, specs

- SETUP.md now deploys conf/sites/ + conf/snippets/levels-common.conf (the
  live config) instead of the retired deploy/levels monolith; HSTS and
  fastcgi_pass references repointed; the historical Oracle Cloud section removed.
- install.service.sh installs all 15 timers (was missing kayak-fetch-osmb and
  kayak-status).
- nginx-editor-env.conf: hostname-aware $site_url (the 2026-05-20 cutover is done).
- engine.py: the SQLite PRAGMA comment reflects the 2 GB CPX11 + swap, not 4 GB.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Default --db to the configured DATABASE_URL (matching how levels resolves it)
so init-db and the metadata load can't target different files when the
operator has set DATABASE_URL. Docstring now uses 'init-db --no-seed' to avoid
duplicate-by-name source rows on import.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs/done/ held 18 completed plans with no README, so a reader couldn't tell
them from in-flight plans — and several are load-bearing (cited by systemd unit
comments, operations.md, slo.md). Added an index that lists each with a
one-line summary and frames them as rationale/provenance, and notes why
PLAN_production_discipline.md stays in docs/ root (live cross-reference target).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- biome: replace the hand-maintained includes allowlist with globs over
  static/ and src/kayak/web/static/ (excluding vendored leaflet.*), so new JS
  is linted automatically instead of silently escaping CI. The allowlist had
  already dropped gradient-profile.js, internal-sort.js, and scroll-indicator.js.
- internal-sort.js: var -> const/let + optional chaining (the noVar debt the
  glob surfaced); 'biome check' is now clean (0 errors).
- lint job: pin ruff==0.15.11 so the format gate can't drift on a new release
  (the churn behind df6e422).
- add .github/dependabot.yml (github-actions / npm / composer, monthly +
  grouped, low-noise for a solo maintainer; pip omitted since uv owns uv.lock).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per docs/one-offs/README's own lifecycle rule (move a script here once its
outcome is captured in the DB -- migration 0046 did): git mv the five run-once
gradient scripts (fetch_dem_tiles, sample_reach_elevations, compute_reach_gradient,
emit_max_gradient_migration, snap_reach_to_channel_min) from scripts/ to
docs/one-offs/ and add their rows to the index.

Repointed the intra-script cross-references and the one live consumer comment
(php/includes/svg_plot.php) to the new path. Migration-header provenance
comments are left as historical (git log --follow resolves them). The geom /
M_TO_FT duplication the review flagged lived in these now-archived scripts, so
it's out of the active tree.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Records the self-contained-rebuild decision in export_metadata.py so a future
maintainer doesn't exclude gradient_profile (or trim huc_name) for size:
`levels init-db --no-seed` + import_metadata.py restores a fully-populated DB
(gradients + basin names) without replaying migration 0046 or re-running
assign-huc.

The reach.csv gradient columns themselves landed via prod's nightly snapshot
(255d6e9), which a drift check confirmed byte-identical to a fresh dev export,
so no separate reach.csv change is needed here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
deploy/levels and conf/levels.nginx were the pre-split monolithic nginx vhost,
retired by the conf/sites/ + conf/snippets/ split and already drifted ~20 lines
from each other; check-config-drift.sh excluded them as retired. Removed both.

Repointed the references that cited them by line number:
- Living docs (posture, controls-map, editor-surface, incident-response) now
  point at the real files: HSTS -> conf/security-headers.conf; access_log ->
  conf/sites/levels-wkcc-org; rate-limit bindings / security.txt / editor
  location blocks -> conf/snippets/levels-common.conf.
- A note atop docs/security/README.md maps deploy/levels -> the split files so
  the point-in-time refs in the dated tier-audit logs and F-1 findings history
  stay interpretable without rewriting the audit record.
- check-config-drift.sh comment updated (removed, not just retired).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
scripts/regression/gauge_pair_linear.py and its test import numpy, but numpy
was only present transitively via the [geo] extra (rasterio -> numpy). A
[dev]-only venv (the documented `pip install -e .[dev]`) therefore lacked it,
so test_gauge_pair_linear hard-failed locally while CI (uv sync --all-extras)
stayed green. Declared numpy in [dev] and refreshed uv.lock so the base test
suite runs without the heavy [geo]/GDAL stack.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The from-scratch VM install surfaced two SETUP.md bugs:

- §3 set only SQLITE_PATH (the PHP variable), pointed inside the repo. But
  `levels` (Python) reads DATABASE_URL, so init-db fell back to its default
  /home/pat/DB/kayak.db -- whose directory didn't exist -> sqlite3
  'unable to open database file'. Now: mkdir /home/pat/DB and set DATABASE_URL +
  SQLITE_PATH to the same prod path (matching the nginx fastcgi_param).
- §4 ran `init-db` (seeded), which leaves ~300 orphan sources and an empty
  site. Switched to init-db --no-seed -> import_metadata.py -> pipeline so the
  gauge_source links load and orphan-check passes (same flow already in
  README/CLAUDE; SETUP.md had been missed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mousebrains
mousebrains merged commit b4bf755 into main May 24, 2026
8 checks passed
@mousebrains
mousebrains deleted the review-fixes branch May 25, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant